refactor: add separate accounts for each policy in e2e tests #2459
Conversation
WalkthroughThis change involves refactoring the Changes
Sequence Diagram(s)No sequence diagrams are necessary as the changes involve refactoring existing functionality to use different accounts, without altering the control flow or introducing new features. Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
kingpinXD
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2459 +/- ##
===========================================
- Coverage 69.05% 68.99% -0.06%
===========================================
Files 308 308
Lines 19192 19209 +17
===========================================
+ Hits 13253 13254 +1
- Misses 5264 5278 +14
- Partials 675 677 +2
|
kingpinXD
changed the title
kingpinXD
requested review from
brewmaster012,
lumtis,
ws4charlie,
skosito,
swift1337 and
fbac
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Outside diff range, codebase verification and nitpick comments (1)
e2e/txserver/zeta_tx_server.go (1)
156-157: Add space after the period in the comment.Ensure there is a space after the period in the comment for better readability.
- // MustGetAccountAddressFromName returns the account address from the given name.It panics on error + // MustGetAccountAddressFromName returns the account address from the given name. It panics on error
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (15)
- changelog.md (1 hunks)
- cmd/zetae2e/config/localnet.yml (1 hunks)
- cmd/zetae2e/local/local.go (1 hunks)
- contrib/localnet/scripts/start-zetacored.sh (2 hunks)
- e2e/config/config.go (5 hunks)
- e2e/e2etests/test_erc20_deposit_refund.go (1 hunks)
- e2e/e2etests/test_eth_deposit_liquidity_cap.go (2 hunks)
- e2e/e2etests/test_migrate_chain_support.go (4 hunks)
- e2e/e2etests/test_pause_zrc20.go (2 hunks)
- e2e/e2etests/test_rate_limiter.go (1 hunks)
- e2e/e2etests/test_update_bytecode_connector.go (1 hunks)
- e2e/e2etests/test_update_bytecode_zrc20.go (1 hunks)
- e2e/runner/setup_zeta.go (2 hunks)
- e2e/txserver/zeta_tx_server.go (1 hunks)
- e2e/utils/zetacore.go (1 hunks)
Additional context used
Path-based instructions (13)
e2e/e2etests/test_update_bytecode_connector.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/e2etests/test_eth_deposit_liquidity_cap.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/e2etests/test_update_bytecode_zrc20.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/e2etests/test_pause_zrc20.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/e2etests/test_erc20_deposit_refund.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/utils/zetacore.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/runner/setup_zeta.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/e2etests/test_rate_limiter.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/e2etests/test_migrate_chain_support.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/config/config.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.cmd/zetae2e/local/local.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/txserver/zeta_tx_server.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.contrib/localnet/scripts/start-zetacored.sh (1)
Pattern
**/*.sh: Review the shell scripts, point out issues relative to security, performance, and maintainability.
Gitleaks
cmd/zetae2e/config/localnet.yml
39-39: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
43-43: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
47-47: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
GitHub Check: codecov/patch
e2e/config/config.go
[warning] 204-208: e2e/config/config.go#L204-L208
Added lines #L204 - L208 were not covered by tests
[warning] 232-232: e2e/config/config.go#L232
Added line #L232 was not covered by tests
[warning] 236-239: e2e/config/config.go#L236-L239
Added lines #L236 - L239 were not covered by tests
[warning] 241-243: e2e/config/config.go#L241-L243
Added lines #L241 - L243 were not covered by tests
[warning] 287-287: e2e/config/config.go#L287
Added line #L287 was not covered by tests
[warning] 291-291: e2e/config/config.go#L291
Added line #L291 was not covered by tests
Additional comments not posted (22)
e2e/e2etests/test_update_bytecode_connector.go (1)
49-53: Ensure correct usage of new policy accounts.The function
MustGetAccountAddressFromNameis used correctly to get the AdminPolicyName account address, and theBroadcastTxfunction is called with the correct policy name. Ensure all instances where theFungibleAdminNamewas used are updated similarly.e2e/e2etests/test_pause_zrc20.go (2)
38-41: LGTM!The change to use the
EmergencyPolicyNamefor pausing the ZRC20 token is consistent with the PR objectives.
109-112: LGTM!The change to use the
OperationalPolicyNamefor unpausing the ZRC20 token is consistent with the PR objectives.e2e/e2etests/test_erc20_deposit_refund.go (1)
39-45: LGTM!The change to use the
OperationalPolicyNamefor refunding the aborted CCTX is consistent with the PR objectives.e2e/utils/zetacore.go (1)
19-21: LGTM!The introduction of new constants for
EmergencyPolicyName,AdminPolicyName, andOperationalPolicyNameis consistent with the PR objectives.e2e/runner/setup_zeta.go (2)
78-78: LGTM!The change to use the
OperationalPolicyNamefor deploying system contracts and ZRC20 tokens on ZEVM is consistent with the PR objectives.
212-219: LGTM!The change to use the
OperationalPolicyNamefor enabling header verification and funding the emissions pool is consistent with the PR objectives.e2e/e2etests/test_rate_limiter.go (1)
174-174: LGTM!The change to use the new
OperationalPolicyNameaccount is correct.e2e/e2etests/test_migrate_chain_support.go (5)
62-63: LGTM!The change to use the new
OperationalPolicyNameaccount is correct.
70-81: LGTM!The change to use the new
OperationalPolicyNameaccount is correct.
99-100: LGTM!The change to use the new
OperationalPolicyNameaccount is correct.
110-111: LGTM!The change to use the new
OperationalPolicyNameaccount is correct.
159-160: LGTM!The change to use the new
OperationalPolicyNameaccount is correct.e2e/config/config.go (3)
46-46: LGTM!The addition of
PolicyAccountsto theConfigstruct is correct.
61-68: LGTM!The removal of
UserFungibleAdminfromAdditionalAccountsis correct.
70-73: LGTM!The addition of
PolicyAccountsstruct is correct.cmd/zetae2e/local/local.go (1)
129-134: LGTM!The change to use the new policy account names is correct.
e2e/txserver/zeta_tx_server.go (1)
158-168: Ensure proper usage of panic in MustGetAccountAddressFromName.Using
panicis generally discouraged in non-test code due to its impact on program stability. However, since this function is intended for test usage only, it is acceptable here. Ensure this function is not used in production code.Ensure the function is used only in test files.
Verification successful
Ensure
MustGetAccountAddressFromNameis used only in test files.The function
MustGetAccountAddressFromNameis used in the following files:
e2e/e2etests/test_update_bytecode_connector.goe2e/e2etests/test_migrate_chain_support.goe2e/e2etests/test_eth_deposit_liquidity_cap.goe2e/e2etests/test_erc20_deposit_refund.goe2e/e2etests/test_pause_zrc20.goe2e/e2etests/test_update_bytecode_zrc20.goThese files are located in the
e2e/e2etests/directory, which suggests they are test files. The function appears to be used only in test files, which aligns with the intended usage.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Ensure `MustGetAccountAddressFromName` is used only in test files. # Test: Search for the function usage. Expect: Only occurances in test files. rg --type go 'MustGetAccountAddressFromName'Length of output: 1700
changelog.md (1)
79-79: LGTM! Addition of separate accounts for each policy in E2E tests.The entry is consistent with the changelog's style and accurately describes the change.
e2e/e2etests/test_eth_deposit_liquidity_cap.go (2)
28-32: Review: UseOperationalPolicyNamefor account retrieval and broadcastingThe new account retrieval method
MustGetAccountAddressFromName(utils.OperationalPolicyName)and the corresponding policy name are used correctly for broadcasting the transaction.
72-77: Review: UseOperationalPolicyNamefor setting liquidity capThe correct account retrieval method and policy name are used for setting the liquidity cap.
e2e/e2etests/test_update_bytecode_zrc20.go (1)
71-75: Review: UseAdminPolicyNamefor updating contract bytecodeThe new account retrieval method
MustGetAccountAddressFromName(utils.AdminPolicyName)and the corresponding policy name are used correctly for updating the contract bytecode.
|
Need #2463 to be merged to fully test the functionality |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- changelog.md (1 hunks)
- e2e/e2etests/test_rate_limiter.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- changelog.md
Additional context used
Path-based instructions (1)
e2e/e2etests/test_rate_limiter.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
Additional comments not posted (1)
e2e/e2etests/test_rate_limiter.go (1)
170-174: UseMustGetAccountAddressFromNamefor better error handling.Instead of checking for errors manually, use
MustGetAccountAddressFromNameto simplify the code and ensure it panics on error, which is appropriate for test setup code.- adminAddr, err := r.ZetaTxServer.GetAccountAddressFromName(utils.OperationalPolicyName) - if err != nil { - return err - } + adminAddr := r.ZetaTxServer.MustGetAccountAddressFromName(utils.OperationalPolicyName)
Description
Closes #2447
How Has This Been Tested?
Summary by CodeRabbit
New Features
Refactor
FungibleAdminNamewith new policy names across tests and configurations.Bug Fixes
Documentation